wp_kses: add support for picture element and srcset attribute on img tags by adamsilverstein · Pull Request #6184 · WordPress/wordpress-develop

adamsilverstein · 2024-02-26T18:43:27Z

Trac ticket: https://core.trac.wordpress.org/ticket/29807

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2024-02-26T19:18:39Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Co-authored-by: Andrew Ozz <[email protected]>

Co-authored-by: Copilot <[email protected]>

Copilot

Pull Request Overview

This PR adds support for the HTML5 <picture> element and srcset attribute on <img> tags to WordPress's KSES filtering system. This enables proper handling of responsive images in content filtering.

Adds <picture> and <source> elements to allowed post tags
Adds srcset and sizes attributes to <img> and <source> elements
Refactors URI sanitization logic to handle multi-URI attributes like srcset

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/wp-includes/kses.php	Adds picture/source elements to allowed tags, adds srcset/sizes attributes, and creates new wp_kses_sanitize_uris function to handle multi-URI attributes
tests/phpunit/tests/kses.php	Adds comprehensive test coverage for img srcset attributes, srcset sanitization, and picture element filtering

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

vishalkakadiya · 2026-02-03T09:12:14Z

@adamsilverstein Can you resolve conflicts on this PR, as the testing team has to apply this patch to QA it? Thanks!

https://core.trac.wordpress.org/ticket/29807#comment:53

adamsilverstein · 2026-02-09T06:38:56Z

@adamsilverstein Can you resolve conflicts on this PR, as the testing team has to apply this patch to QA it? Thanks!

https://core.trac.wordpress.org/ticket/29807#comment:53

yes, I will work on that.

juanmaguitar · 2026-03-13T16:54:28Z

@adamsilverstein thanks for working on this ticket!
Do you think you'll be able to address the requested changes in time for WP 7.0 RC1 (happening in ~1 week)?

# Conflicts: # src/wp-includes/kses.php

adamsilverstein · 2026-03-13T17:49:28Z

@adamsilverstein thanks for working on this ticket! Do you think you'll be able to address the requested changes in time for WP 7.0 RC1 (happening in ~1 week)?

I have resolved the merge conflicts and am looking at adding additional tests. I'll also review feedback on the Trac ticket. Also fine punting this to 7.1 early.

github-actions · 2026-03-13T17:49:46Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props adamsilverstein, azaozz, vishalkakadiya, juanmaguitar.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Tests currently fail, documenting three bugs: - CDN resizer URLs with commas in paths are incorrectly split by wp_kses_sanitize_uris() naive comma splitting - Original spacing around commas is not preserved - decoding and fetchpriority attrs stripped from img tags Also adds passing coverage tests for wp_kses_uri_attributes, wp_kses_one_attr srcset handling, source/picture element edge cases, and the URI attributes filter hook.

These performance attributes are commonly added by WordPress core (wp_img_tag_add_loading_optimization_attrs) and should not be stripped when content passes through KSES filtering.

CDN image resizers (e.g. Cloudflare) use commas in URL paths like cdn-cgi/image/format=auto,quality=80,width=412/... which were incorrectly split by the naive comma-based preg_split in wp_kses_sanitize_uris(). This rewrites the splitting to use the srcset descriptor pattern (e.g. 480w, 2x) as entry boundaries, preserving commas within URLs. Also preserves original whitespace around separators instead of normalizing with implode.

WordPress coding standards require aligned double arrows in multi-line arrays. Adjusts spacing so all arrows in the data provider align with the longest key.

adamsilverstein · 2026-03-14T00:29:13Z

I added some fixes and additional tests.

@SInCE

Expand test coverage for ticket #29807 with edge cases: CDN URLs with gravity=0.5x0.5 parameters, decimal pixel density descriptors, picture element type fallbacks and art direction, protocol-relative URLs, and descriptor-less final srcset entries. Update @SInCE to 7.1.0 per milestone.

adamsilverstein · 2026-04-23T22:22:23Z

Refreshed against current trunk (clean merge, CI green). Given 7.0 RC1 has passed, now targeting 7.1.

westonruter · 2026-04-24T04:53:19Z

+			'srcset'        => true,
+			'usemap'        => true,
+			'width'         => true,
+			'sizes'         => true,


This attribute seems it should be moved up to appear before src to maintain alphabetic ordering.

westonruter · 2026-04-24T04:54:07Z

+			'srcset' => true,
+			'type'   => true,
+			'media'  => true,
+			'sizes'  => true,


westonruter · 2026-04-24T04:56:54Z

+ * @param string   $attrname          The attribute name to test.
+ * @param string   $attrvalue         The attribute value to sanitize.
+ * @param string[] $allowed_protocols Array of allowed URL protocols.
+ * @param string[] $multi_uri         Optional. Attributes that can contain multiple URIs. Default is array( 'srcset' ).
+ * @return string Sanitized attribute value.
+ */
+function wp_kses_sanitize_uris( $attrname, $attrvalue, $allowed_protocols, $multi_uri = array( 'srcset' ) ) {
+	$uris = wp_kses_uri_attributes();
+
+	if ( ! in_array( strtolower( $attrname ), $uris, true ) ) {
+		return $attrvalue;
+	}
+
+	if ( in_array( strtolower( $attrname ), $multi_uri, true ) ) {


Let's add underscores to some vars, like $attr_name and $attr_value, and then let's add attrs to others, including $multi_uri_attrs and $uri_attrs. Otherwise, I was reading this and I was confused why attribute names would be among URIs.

westonruter · 2026-04-24T04:58:03Z

+ * @param string[] $multi_uri         Optional. Attributes that can contain multiple URIs. Default is array( 'srcset' ).
+ * @return string Sanitized attribute value.
+ */
+function wp_kses_sanitize_uris( $attrname, $attrvalue, $allowed_protocols, $multi_uri = array( 'srcset' ) ) {


I think these can add PHP type hints as well for the params and the return value.

westonruter · 2026-04-24T04:58:28Z

+				if ( preg_match( '/^(\s*)(.*?)(\s+\d+[wx])?\s*$/i', $entry, $m ) ) {
+					$leading_ws = $m[1];
+					$url        = $m[2];
+					$descriptor = isset( $m[3] ) ? $m[3] : '';


Suggested change

$descriptor = isset( $m[3] ) ? $m[3] : '';

$descriptor = $m[3] ?? '';

westonruter · 2026-04-24T05:00:33Z

+		 * Instead, split on: whitespace + descriptor + comma (+ optional whitespace).
+		 * This correctly identifies only the commas that separate srcset entries.
+		 */
+		$parts  = preg_split( '/(\s+\d+[wx]\s*,\s*)/i', $attrvalue, -1, PREG_SPLIT_DELIM_CAPTURE );


Suggested change

$parts = preg_split( '/(\s+\d+[wx]\s*,\s*)/i', $attrvalue, -1, PREG_SPLIT_DELIM_CAPTURE );

$parts = array_filter( (array) preg_split( '/(\s+\d+[wx]\s*,\s*)/i', $attrvalue, -1, PREG_SPLIT_DELIM_CAPTURE ) );

westonruter · 2026-04-24T05:02:23Z

+		for ( $i = 0, $len = count( $parts ); $i < $len; $i++ ) {
+			if ( preg_match( '/^\s+\d+[wx]\s*,\s*$/i', $parts[ $i ] ) ) {
+				// This is a delimiter: space + descriptor + comma. Append it as-is.
+				$result .= $parts[ $i ];
+			} else {
+				// This is a URL (possibly with a trailing descriptor for the last entry).
+				$entry = $parts[ $i ];


Using foreach can simplify this I believe.

Suggested change

for ( $i = 0, $len = count( $parts ); $i < $len; $i++ ) {

if ( preg_match( '/^\s+\d+[wx]\s*,\s*$/i', $parts[ $i ] ) ) {

// This is a delimiter: space + descriptor + comma. Append it as-is.

$result .= $parts[ $i ];

} else {

// This is a URL (possibly with a trailing descriptor for the last entry).

$entry = $parts[ $i ];

foreach( $parts as $entry ) {

if ( preg_match( '/^\s+\d+[wx]\s*,\s*$/i', $entry ) ) {

// This is a delimiter: space + descriptor + comma. Append it as-is.

$result .= $entry;

} else {

// This is a URL (possibly with a trailing descriptor for the last entry).

if ( preg_match( '/^(\s*)(.*?)(\s+\d+[wx])?\s*$/i', $entry, $m ) ) {

westonruter · 2026-04-24T05:02:51Z

+			} else {
+				// This is a URL (possibly with a trailing descriptor for the last entry).
+				$entry = $parts[ $i ];
+				if ( preg_match( '/^(\s*)(.*?)(\s+\d+[wx])?\s*$/i', $entry, $m ) ) {


Let's use $matches instead of $m to avoid abbreviation.

attempt to bring 29807.8.diff up to date with trunk

2bf9709

Merge remote-tracking branch 'upstream/trunk' into pr/6184

11336a3

azaozz reviewed Mar 11, 2024

View reviewed changes

Comment thread src/wp-includes/kses.php Outdated

azaozz reviewed Mar 11, 2024

View reviewed changes

Comment thread src/wp-includes/kses.php Outdated

azaozz reviewed Mar 11, 2024

View reviewed changes

Comment thread src/wp-includes/kses.php Outdated

azaozz reviewed Mar 11, 2024

View reviewed changes

Comment thread tests/phpunit/tests/kses.php Outdated

azaozz and others added 12 commits March 11, 2024 16:20

Fix coding standards spaces and methods visibility

05dcd69

More CS empty space fixes.

499dc82

Merge branch 'trunk' into ticket/29807

7a75eba

Apply suggestion from @azaozz

69312e7

Co-authored-by: Andrew Ozz <[email protected]>

string in_array test

3c6defd

expand test slightly

c88673d

Cleanup; move multi_uri to parameter

022def5

phpcbf

321e441

add sizes attribute to test

c7c63dd

enable sizes

e3c1684

Improve doc block

3862627

test clean up

4cb049a

adamsilverstein requested a review from Copilot August 17, 2025 16:30

This comment was marked as outdated.

Sign in to view

adamsilverstein and others added 3 commits August 17, 2025 10:35

Update tests/phpunit/tests/kses.php

ca94908

Co-authored-by: Copilot <[email protected]>

Update src/wp-includes/kses.php

49f1ba6

Co-authored-by: Copilot <[email protected]>

remove unused $uris

2b5706c

adamsilverstein requested review from azaozz and Copilot August 17, 2025 16:41

Copilot AI reviewed Aug 17, 2025

View reviewed changes

Comment thread src/wp-includes/kses.php Outdated

Comment thread src/wp-includes/kses.php Outdated

adamsilverstein added 3 commits August 17, 2025 10:56

Add additional test cases.

d116c8e

Merge branch 'trunk' into ticket/29807

fbeed82

phpcbf

f9e002c

adamsilverstein force-pushed the ticket/29807 branch 2 times, most recently from c402114 to f9e002c Compare February 10, 2026 00:48

Merge remote-tracking branch 'origin/trunk' into ticket/29807

80521e4

# Conflicts: # src/wp-includes/kses.php

adamsilverstein marked this pull request as ready for review March 13, 2026 17:49

adamsilverstein added 5 commits March 13, 2026 13:29

Add decoding and fetchpriority to img allowed attrs

f11aeb2

These performance attributes are commonly added by WordPress core (wp_img_tag_add_loading_optimization_attrs) and should not be stripped when content passes through KSES filtering.

Fix array double arrow alignment in kses tests

399aa30

WordPress coding standards require aligned double arrows in multi-line arrays. Adjusts spacing so all arrows in the data provider align with the longest key.

Merge branch 'trunk' into ticket/29807

e5488a6

adamsilverstein added 4 commits March 14, 2026 06:36

Fix array double arrow alignment in new tests

9bb1ebf

Merge branch 'trunk' into ticket/29807

2bdc1f9

Merge branch 'trunk' into ticket/29807

61881bf

adamsilverstein requested a review from westonruter April 23, 2026 23:06

westonruter reviewed Apr 24, 2026

View reviewed changes

	$descriptor = isset( $m[3] ) ? $m[3] : '';
	$descriptor = $m[3] ?? '';

	$parts = preg_split( '/(\s+\d+[wx]\s,\s)/i', $attrvalue, -1, PREG_SPLIT_DELIM_CAPTURE );
	$parts = array_filter( (array) preg_split( '/(\s+\d+[wx]\s,\s)/i', $attrvalue, -1, PREG_SPLIT_DELIM_CAPTURE ) );

Conversation

adamsilverstein commented Feb 26, 2024

Uh oh!

github-actions Bot commented Feb 26, 2024

Test using WordPress Playground

Some things to be aware of

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

vishalkakadiya commented Feb 3, 2026

Uh oh!

adamsilverstein commented Feb 9, 2026

Uh oh!

juanmaguitar commented Mar 13, 2026

Uh oh!

adamsilverstein commented Mar 13, 2026

Uh oh!

github-actions Bot commented Mar 13, 2026

Uh oh!

adamsilverstein commented Mar 14, 2026

Uh oh!

adamsilverstein commented Apr 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants